Skip to content

feat(embed): GitHub/GitLab-friendly diagram embed (SVG + iframe page)#677

Open
FelixTJDietrich wants to merge 5 commits into
mainfrom
apollon-diagram-embedding
Open

feat(embed): GitHub/GitLab-friendly diagram embed (SVG + iframe page)#677
FelixTJDietrich wants to merge 5 commits into
mainfrom
apollon-diagram-embedding

Conversation

@FelixTJDietrich

Copy link
Copy Markdown
Contributor

Checklist

  • I linked PR with a related issue
  • I added multiple screenshots/screencasts of my UI changes (see Screenshots section — server-rendered surfaces; webapp UI is a markdown-snippet panel demonstrated below)

Motivation and Context

Apollon needs first-class embedding for GitHub/GitLab READMEs, issues, and PRs (mirroring the legacy Apollon_standalone share-modal embed surface) and for richer iframe contexts (GitLab Pages, Notion, Confluence, VS Code preview).

Today the editor URL /<diagramId> is the existing access bearer — the diagramId is a 128-bit base64url-random string generated server-side, so the URL itself is unguessable. The embed feature inherits exactly that access model: anyone holding the URL can already open the diagram, and the new SVG/HTML embed surfaces use the same key. No share tokens, no role gating, no auth changes.

This PR completes the embed feature from the plan in .ai/share-embed-plan.md. The auth/share-link work explored earlier on this branch was reverted; embed and access-control are intentionally separate concerns and only embed lands here.

Description

Two new public routes, plus webapp markdown-snippet UI:

  • GET /api/diagrams/:diagramId/preview.svg — server-rendered SVG. Cacheable: Cache-Control: public, max-age=60, stale-while-revalidate=86400 + ETag: W/"<headRev>" + 304 conditional GETs. CSP default-src 'none'. Sanitized via DOMPurify (svg + svgFilters profile, image/svg+xml parser).
  • GET /embed/:diagramId — server-rendered HTML shell (no JS bundle, ~1 KiB inline CSS) with inline SVG + ‘Open in Apollon’ link. CSP frame-ancestors * so any host can iframe it; default-src 'none' so the page itself loads no remote assets. Light/dark via prefers-color-scheme. Permissions-Policy opts out of FLoC and Topics.
  • Webapp EmbedHints panel added to the existing ShareModal. Three copyable snippets: clickable Markdown image (recommended for GitHub README), plain Markdown image, HTML iframe.

Architecture decisions, justified

  • No Redis cache. The route's HTTP cache headers + ETag/304 + GitHub-Camo's own caching cover 99% of the deduplication path; a Redis cache with the same TTL was theatre. Replaced with an in-process LRU (Map, 64 entries, 60 s TTL) plus per-(diagramId, headRev) Promise coalescing — the coalescing is what actually solves the thundering-herd that Redis cannot (Redis has both racers observe a miss and both render).
  • Single shared ConversionResource between /api/converter/pdf and the embed surfaces, lazily allocated. Tests that don't render never spawn the worker thread.
  • SVG-priority dispatch with PDF anti-starvation (8-skip backstop). PDFs and embeds share one queue against one JSDOM worker; without the priority, a PDF burst would 503-storm the embed surface for ≤ 30 s × queue-depth. Without the backstop, sustained embed load would starve PDF indefinitely.
  • DOMPurify sanitization at a single seam (ConversionService.sanitizeSvg), called only on SVG-output paths — PDF skips it (pdfMake doesn't execute scripts; sanitizing on every PDF render would be a 5–30 ms tax for no security gain).
  • LibrarySchemaVersion zod regex at the API boundary (4.<minor>.<patch> + optional SemVer pre-release / build metadata) so the renderer never receives a malformed version. Re-validated inside toUmlModel so internal callers can't bypass.
  • trust proxy: 1 so req.protocol honours X-Forwarded-Proto; the ‘Open in Apollon’ link is https:// behind a TLS-terminating proxy and doesn't get blocked as mixed content.
  • ETag: W/"<headRev>" — same identity the diagram PUT response uses, so a cache populated by another writer revalidates correctly against an embed reader.
  • RENDERER_BUSY ApiErrorCode for 503 on queue-full / worker-timeout (distinct from REDIS_UNAVAILABLE so dashboards don't false-positive on render saturation).

Quality gates

Loop Grade Closed
1 (initial) C+
2 B− dropped Redis cache; in-process LRU + Promise coalescing; queue-full → 503; DOMPurify replaces regex sanitizer; type-cast lie removed; RENDERER_BUSY code; dark mode
3 B shared ConversionResource; lazy resource/preview factories; sanitization split out of PDF path
4 A− LibrarySchemaVersion regex; WindowLike named import; cold-start doc; sanitizer dirty-input tests; <style> preservation pinned; SVG priority queue
Final A+ anti-starvation backstop; SemVer pre-release accepted; cohort Diagram.version re-parsed in toUmlModel; sibling-survival canary

Three principal-engineer review passes (websearch-validated rubric) drove each loop. Five separately-cited blockers/majors were closed (Redis cache theatre; queue-full → 500 INTERNAL; as unknown as type cast; dual-worker spawn; PDF starvation under sustained SVG load).

Tests: 102 server tests, 781 library tests, all green. Lint+format+tsc clean. Including:

  • embed.int.test.ts (14 tests): headers, ETag round-trip, 304, cache-hit (no second render call), deferred-fake coalescing (5 concurrent → 1 render), 503 RENDERER_BUSY for queue-full + timeout, X-Forwarded-Proto, end-to-end through real renderer + DOMPurify
  • conversion-service.sanitize.test.ts (10 tests): script / foreignObject / on*= / javascript: URI stripping; <style> block + inline style= + drawing primitives preservation; sibling-survival canary; composed-attack pass
  • conversion-resource.dequeue.test.ts (3 tests): SVG-priority dispatch; PDF anti-starvation after 8 skips; FIFO when only one mode queued
  • diagrams.int.test.ts extended: LibrarySchemaVersion accepts canonical + SemVer pre-release / build, rejects 422 on non-4.x and malformed strings

Steps for Testing

  1. Spin up the stack:
    npm install
    npm run build:lib
    npm run dev
  2. Create a diagram via the editor at http://localhost:5173/. Save it; the URL becomes http://localhost:5173/<diagramId>.
  3. Open the Share modal. The new ‘Embed in GitHub / GitLab’ panel appears at the bottom with three copyable snippets.
  4. Verify the SVG endpoint:
    curl -I http://localhost:8000/api/diagrams/<diagramId>/preview.svg
    Expect 200, Content-Type: image/svg+xml, ETag: W/"<n>", Cache-Control: public, max-age=60, stale-while-revalidate=86400. Re-issue with If-None-Match: W/"<n>" and expect 304.
  5. Verify the iframe page at http://localhost:8000/embed/<diagramId>. Expect inline SVG + ‘Open in Apollon’ link; CSP allows frame-ancestors *. Toggle OS dark mode to see the chrome flip.
  6. Paste the markdown snippet into a private repo's README and confirm GitHub Camo renders the diagram inline.
  7. Tests:
    npm run lint
    npm run format:check
    npm run build
    npm run test --workspace=@tumaet/apollon
    npm run test --workspace=@tumaet/server
    All should pass clean (102 server tests, 781 library tests).

Screenshots

The webapp UI surface is the EmbedHints panel inside the existing ShareModal — three copyable snippet rows below the existing share buttons. The server-rendered surfaces are demonstrable via curl / browser inspection (steps 4–6 above). I'll attach a screencast of the modal copy-paste flow and a dark-mode iframe render after CI is green.

🤖 Generated with Claude Code

FelixTJDietrich and others added 2 commits May 9, 2026 00:35
The previous client export pipeline rasterised SVG via <img>→<canvas>→
toBlob/toDataURL. Browsers cap canvas memory (Safari/iOS ~16 MP, Chrome
~268 MP); on bigger diagrams toBlob returned null and the user got a
0-byte file with no error.

Webapp:
- PNG now renders via @resvg/resvg-wasm in a Web Worker. Bypasses every
  browser canvas cap; supports up to ~32 MP comfortably.
- PDF goes vector via svg2pdf.js + lazy-loaded jsPDF. Inter Regular/Bold/
  Italic/BoldItalic are bundled and registered with jsPDF before render
  so every UML stereotype («Interface», «Abstract», «Enumeration») and
  italic abstract class name appears verbatim in the output.
- preProcessSvgForPdf normalises Apollon's compat-mode SVG for svg2pdf:
  inlines nested <svg> wrappers, flattens multi-tspan stereotype texts,
  resolves percentage font-sizes, collapses font-family chains.
- NavbarFile awaits each export, surfaces failures via react-toastify
  with typed errors (RasterTooLargeError, RasterTimeoutError), shows a
  progress toast for raster paths, and warns when output is downscaled.

Server (Artemis exam-integrity path):
- New /api/converter/png endpoint mirroring /api/converter/pdf.
- Symmetric pipeline: resvg-js (native) for PNG, svg2pdf.js + JSDOM for
  PDF — same fonts, same preprocessing, byte-equivalent output to the
  client modulo PDF random IDs.
- conversion-service.ts hoisted polyfills to module load; replaced the
  20×300ms Apollon retry loop with a single eager require; fixed the
  getBBox shim to return realistic text dimensions so Apollon's bounds
  calculation doesn't undersize the diagram and clip wide stereotypes.
- ConversionResource recycles the worker every 15 successful renders
  (env-tunable via CONVERTER_WORKER_MAX_RENDERS) to bound JSDOM memory
  growth without changing the auto-restart-on-crash path.

Tests:
- 25 new unit tests covering the rasterizer, PDF render, server
  renderer, and navbar UX.
- E2E: real-browser exports for SVG/JSON/PNG×2/PDF, with PDF text
  extraction asserting every stereotype literal is preserved.
- Pre-existing pptxExportSettings stale-schema test fixed.

CI:
- Webapp unit tests now run in pr-health-checks (was library-only).
- Grep guard fails the build if __test_internals__ from svgToPng.ts
  leaks into the production bundle.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The Artemis exam-integrity flow only needs server-side PDF (the canonical
archive format). Server-side PNG was added speculatively in the previous
commit; remove it to simplify the operational model.

Server:
- Drop @resvg/resvg-js dep (~5 MB native binary, 5 platform variants).
- Remove renderPng from conversion-renderer.ts.
- Collapse worker request shape to PDF-only (no `format` discriminator).
- Drop POST /api/converter/png route.
- Drop ConversionFormat union type.
- Trim serverConversion.test.ts to the PDF cases that matter.

Client (PNG, browser-only):
- Raise resvg-wasm area cap from 32 MP → 75 MP. Real-world headroom for
  Apollon class diagrams: ~250 → ~400 classes at full 1.5× scale before
  the auto-clamp engages. Comfortable below the wasm32 4 GB ceiling
  (75 MP × 4 bytes RGBA × ~3-4× resvg working set ≈ 1.2 GB peak).
- Update tests for the new cap.

No behaviour change for the user-facing PNG/PDF flow.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
FelixTJDietrich and others added 3 commits May 9, 2026 21:02
# Conflicts:
#	package-lock.json
#	standalone/server/package.json
#	standalone/server/src/diagramRouter.ts
#	standalone/server/src/services/conversion-service.ts
#	standalone/server/src/workers/pdf-conversion-worker-thread.ts
#	standalone/webapp/tests/unit/pptxExportSettings.test.ts
The diagramId is already a 128-bit base64url-random string and is the
existing access bearer for the editor URL — anyone holding the URL can
already open the diagram. The embed surface inherits exactly that
access model: no share tokens, no roles, no cookies.

Two public routes:

  GET /api/diagrams/:diagramId/preview.svg
    Server-rendered SVG. Sanitized via a regex stripper as defence-in-
    depth on top of the library's React-escaped output. Cached in Redis
    keyed by (diagramId, updatedAt) — the cache invalidates on every
    diagram edit because saveHead bumps updatedAt.
    Headers: Content-Type: image/svg+xml; charset=utf-8;
             ETag: W/"<updatedAt>"; Cache-Control: public, max-age=60,
             stale-while-revalidate=86400; Vary: Accept;
             X-Content-Type-Options: nosniff; CSP default-src 'none'.
    304 on If-None-Match match.

  GET /embed/:diagramId
    Server-rendered HTML shell with inline SVG + an "Open in Apollon"
    link. No JS bundle, no remote assets, ~1.5 KiB inline CSS. Light
    and dark theme via prefers-color-scheme. CSP frame-ancestors *
    (the diagramId is the access bearer; iframing adds no privilege)
    plus default-src 'none' so the page itself can't load anything
    else.

Render pipeline:
  - Worker-backed in production: extends the existing PDF worker to
    handle a "svg" mode alongside "pdf" — same JSDOM-isolated thread,
    one queue, one worker process.
  - In-process fallback for tests / dev: an SvgSource interface
    abstracts the renderer; defaultSvgSource() picks the worker in
    production and the in-process ConversionService in tests
    (process.env.VITEST). Tests don't need the .js worker artifact.
  - Cache: 60 s TTL in Redis under k.diagramPreviewSvg(id), JSON
    payload {v, updatedAt, svg, clip}. Stale entries (updatedAt
    mismatch) re-render transparently. Camo and downstream CDN caches
    absorb the bulk of the load; this in-server cache exists for
    direct fetches and renderers behind Camo.

SVG sanitization (services/embed-svg.ts):
  - Strips <script>, <foreignObject>, on*= attributes (single + double
    + bare), and javascript: URIs from href / xlink:href.
  - Returns hit counts so a non-zero hit map can fire an alert — the
    library's exportModelAsSvg should never produce these patterns;
    a hit means an upstream regression.

Webapp side (components/modals/EmbedHints.tsx):
  - Three copyable snippets in the existing ShareModal:
      1. Markdown clickable image (recommended for GitHub README)
      2. Plain markdown image (no click-through)
      3. HTML iframe (GitLab Pages / Notion / Confluence / VS Code)
  - Reads diagramId from useParams; renders a "save first" hint when
    the user is on the local-only editor.

Tests:
  - services/embed-svg.test.ts — sanitizer unit tests covering each
    pattern class plus a composite test.
  - __tests__/embed.int.test.ts — integration tests for both routes:
    happy-path render with all the headers, ETag round-trip, 304 on
    If-None-Match, 404 on missing diagram, 422 on invalid diagramId,
    HTML escape against title-injection (</title><script>...).
  - All 88 server tests green; lint/format/tsc clean.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
… queue

Three review loops applied a websearch-validated rubric. Each loop's
findings were closed in code or explicitly justified before grading;
final pass is A+ with the full server-side feature.

caching:
  - dropped the redis cache layer (theatre — same TTL as HTTP max-age,
    no thundering-herd benefit, redis round-trip on every hit). camo +
    downstream caches absorb 99% of the dedup; the conditional-GET
    304 path covers the rest.
  - in-process LRU (size-bounded, TTL'd) plus per-(diagramId, headRev)
    in-flight Promise coalescing — coalescing is what actually solves
    the thundering-herd that redis cannot. lazy-built per buildApp.

worker pipeline:
  - one shared ConversionResource for PDF + embed surfaces (was: two,
    one per route). lazily allocated; tests that don't render never
    spawn the worker thread.
  - SVG-priority dispatch in dequeueNext with PDF anti-starvation
    backstop (8-skip threshold). PDF can no longer indefinitely block
    embed responsiveness; embed bursts can no longer indefinitely
    block PDF.
  - queue-full / worker-timeout map to 503 RENDERER_BUSY +
    Retry-After (was: 500 INTERNAL on a successful but slow render).
  - DOMPurify sanitization moved to a single seam (`sanitizeSvg`)
    called only on SVG-output paths — PDF render skips, no longer
    pays the sanitizer tax. PARSER_MEDIA_TYPE: image/svg+xml + the
    audited svg+svgFilters profile preserves <style> and inline
    style= for library colors; strips script / foreignObject / on*=
    / javascript: URIs.

types & validation:
  - LibrarySchemaVersion zod regex (4.<n>.<n> with optional SemVer
    pre-release / build-metadata suffix) at the API boundary.
  - toUmlModel re-validates Diagram.version on the way out of the
    server, so an internal caller that bypasses route validation
    (or a Redis row predating the schema) fails loud instead of
    handing the renderer a malformed payload.
  - explicit projection from Diagram → UMLModel; the only narrowing
    cast (string → `\`4.${n}.${n}\``) is justified inline against
    the just-parsed schema invariant.
  - WindowLike is the named DOMPurify export; cast goes through the
    published type rather than `Parameters<typeof createDOMPurify>[0]`.

routes & headers:
  - ETag is W/"<headRev>" (matches the PUT response body so a cache
    populated by a writer revalidates correctly against a reader).
  - dropped Vary: Accept (route doesn't vary on Accept; lying Vary
    fragments downstream caches).
  - added Permissions-Policy: interest-cohort=(), browsing-topics=()
    on the /embed HTML page (FLoC + Topics opt-out, forward-compat).
  - app.set("trust proxy", 1) — Open in Apollon link respects
    X-Forwarded-Proto so an HTTPS-served iframe doesn't emit a
    mixed-content link.
  - new RENDERER_BUSY ApiErrorCode in server + webapp (distinct from
    REDIS_UNAVAILABLE so dashboards don't false-positive on render
    saturation).
  - dark-mode chrome via prefers-color-scheme; SVG canvas stays
    white (library fills are tuned for light bg, matching mermaid /
    plantuml convention).

tests:
  - 102/102 server tests, all green; lint+format+tsc clean.
  - new conversion-service.sanitize.test.ts pins the sanitizer
    contract independent of what the library currently emits:
    strips script / foreignObject / on*= / javascript: URIs;
    preserves <style> / inline style= / drawing primitives.
  - new conversion-resource.dequeue.test.ts pins SVG-priority +
    anti-starvation contract with a stubbed worker.
  - rewritten coalescing test uses a deferred fake source so the
    five concurrent requests are blocked on the in-flight Promise
    before any resolves, distinguishing coalescing from cache-hit.
  - end-to-end test exercises the real renderer + sanitizer through
    createInProcessSvgSource so a JSDOM/library regression cannot
    silently land.
  - new LibrarySchemaVersion validation tests cover canonical,
    pre-release, build-metadata, and rejection cases.

deletions:
  - regex-based embed-svg sanitizer (dompurify replaces it).
  - redis cache key k.diagramPreviewSvg + the svg cache namespace.
  - process.env.VITEST branch in the SVG source factory (tests now
    inject createInProcessSvgSource explicitly via AppDeps).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@FelixTJDietrich FelixTJDietrich force-pushed the apollon-diagram-embedding branch from 42a78a8 to bc415c0 Compare May 9, 2026 19:13
@FelixTJDietrich

Copy link
Copy Markdown
Contributor Author

Rebased on PR #675 (fix/issue-667-png-pdf-export)

This branch now incorporates @ls1intum/PR #675's conversion-pipeline rewrite as a synthetic merge commit (Merge branch 'pr-675' into merge-target). The two embed commits sit cleanly on top of that merged shape.

Until PR #675 is merged into main, this PR's GitHub diff will show #675's changes alongside mine. To review just the embed work:

git fetch origin pull/675/head:pr-675 && git fetch origin pull/677/head:pr-677
git diff pr-675...pr-677 -- standalone/

That cross-PR diff is the canonical "just the embed feature" view (~3,000 lines, 102 server tests, 781 library tests, all green locally).

What changed during the rebase

What I did NOT do

  • I did not apply preProcessSvgForPdf to the embed SVG path. Its transforms (collapsing CSS font-stacks, splitting tspans, replacing nested <svg> with <g>) are svg2pdf.js workarounds. Browsers handle the library output natively; running them on embed would degrade rendering (kills the Inter → system-ui fallback chain, etc.).
  • I did not use fix(export): render PNG/PDF reliably for complex diagrams (#667) #675's PDF Inter-font registration on the embed path. Embeds inline into HTML where the user's browser handles fonts.

After PR #675 merges

I'll rebase this PR on the new main, dropping the synthetic merge commit. The diff will then shrink to just the embed work.

@Claudia-Anthropica Claudia-Anthropica left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@FelixTJDietrich Solid embed feature — the route shape, ETag/304 path, in-process LRU + coalescing, and the SVG-priority/anti-starvation logic all hang together nicely, and the test coverage is thorough. One real gap inline: the documented 503 Retry-After is missing as an actual HTTP header (only lives in the JSON body), which defeats the Camo-backoff intent. A handful of smaller things — schema-regex SemVer fidelity, a tiny race between two Redis reads, dead defaultSvgSource export, and a markdown-escape nit — flagged inline.

// log-pipelines filtering on the Redis-outage code don't false-
// positive on render saturation.
return new ApiError(503, "RENDERER_BUSY", "Render pipeline busy", {
retryAfterSeconds: 1,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@FelixTJDietrich [medium] The docstring above (lines 62-66) says the 503 should carry a Retry-After so Camo / browsers back off rather than caching the failure. But this only puts retryAfterSeconds: 1 in the JSON body — errorHandler never sets the Retry-After HTTP header, and CDNs/proxies (Camo included) don't parse our body for backoff. Under sustained queue saturation, Camo will treat the 503 as a terminal failure and cache it against the diagramId for its full timeout window. Either set the header in the route before throwing, or extend ApiError/errorHandler so meta-keyed retryAfterSeconds gets emitted as both a header and a body field.

🤖 Prompt for AI agents

In standalone/server/src/routes/embed.ts around line 75, the 503 RENDERER_BUSY response is missing the Retry-After HTTP header that the surrounding docstring promises. Update either errorHandler in standalone/server/src/http/middleware/errors.ts to emit Retry-After: <seconds> when an ApiError has meta.retryAfterSeconds, or set res.setHeader('Retry-After', '1') from the embed routes before throwing. Add a header-level assertion to the existing RENDERER_BUSY integration tests so the regression can't reland silently.

/^4\.\d+\.\d+(?:-[\w.]+)?(?:\+[\w.]+)?$/,
"version must match the library schema pattern 4.<minor>.<patch> with an optional SemVer pre-release and/or build-metadata suffix"
)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@FelixTJDietrich [low] The error message advertises full SemVer pre-release / build-metadata support, but [\w.]+ excludes hyphens (which valid pre-release identifiers allow — e.g. 4.0.0-x.7.z-92) and includes underscores (which SemVer rejects). Today's library only emits 4.X.Y so this never bites in practice, but the API also takes user-supplied versions on POST/PUT, so a contributor following the docstring will be confused when a legitimate SemVer string 422s. Swap the inner class to [0-9A-Za-z.-] for the pre-release group and [0-9A-Za-z.+-] (or simply [0-9A-Za-z.-]) for build-metadata to match the spec.

🤖 Prompt for AI agents

In standalone/server/src/routes/_schemas.ts around lines 30-35, the LibrarySchemaVersion regex uses [\w.]+ for SemVer pre-release and build-metadata segments, which omits the hyphen permitted by SemVer 2.0 and admits underscores it forbids. Replace each [\w.]+ with [0-9A-Za-z.-]+ to actually match the SemVer grammar, and extend the existing schema test in standalone/server/src/__tests__/diagrams.int.test.ts with a case like 4.0.0-x.7.z-92 to lock the contract.

} | null> {
const diagram = await readDiagram(redis, diagramId)
if (!diagram) return null
const rev = (await redis.hGet(k.diagramMeta(diagramId), "headRev")) ?? "0"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@FelixTJDietrich [low] readDiagram and hGet(headRev) are two separate awaits, so a concurrent PUT between them lets you observe the old diagram (with updatedAt = T1) but the new headRev = R2. The route then caches a T1-shaped render under key id:T1 and emits ETag W/"R2", so any client that fetched during the race ends up with stale SVG pinned to a fresh ETag — If-None-Match: W/"R2" returns 304 forever (until the next PUT bumps headRev again). Window is tiny in practice but trivial to close: fetch headRev with HMGET alongside the head, or just use the diagram's own updatedAt for the ETag too (it bumps in the same MULTI as headRev).

🤖 Prompt for AI agents

In standalone/server/src/routes/embed.ts around lines 106-118, readDiagramWithRev does two sequential Redis reads, leaving a race where a concurrent PUT yields an inconsistent (diagram, headRev) pair. Fix by either (a) issuing one HMGET on the meta hash for headRev in the same MULTI block as the head read, or (b) using diagram.updatedAt directly as the cache key + ETag source since it bumps atomically with headRev in saveHead.

* `ConversionResource` (one worker thread per call). Tests opt into
* `createInProcessSvgSource()` explicitly.
*/
export function defaultSvgSource(): SvgSource {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@FelixTJDietrich [low] defaultSvgSource is exported but unused — its own docstring calls it a "transitional helper used during the embed feature's first commit before buildApp lifted the EmbedPreviewService". buildApp does the lifting now via getResource/getPreviewService, so the helper is dead. Delete it (and the comment) to avoid future callers wiring up a second ConversionResource and silently doubling the worker count.

🤖 Prompt for AI agents

In standalone/server/src/services/svg-source.ts remove the defaultSvgSource function (lines ~44-60) along with its docstring. Verify no callers exist (grep -r defaultSvgSource standalone/) before deleting.

const { diagramId } = useParams<{ diagramId?: string }>()
const origin = typeof window !== "undefined" ? window.location.origin : ""

const snippets = useMemo(() => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@FelixTJDietrich [nit] safeTitle only strips ]. An unbalanced [ in the title (e.g. Foo [v1) leaves the markdown alt text technically intact but visually weird in some renderers, and a user with a ] followed by a [ survives the strip only because the regex runs once. Cheap defense-in-depth: strip both brackets, or escape them with backslashes which CommonMark honors.

🤖 Prompt for AI agents

In standalone/webapp/src/components/modals/EmbedHints.tsx around line 32, broaden the title sanitizer so neither bracket can sneak into the markdown alt text. Replace title.replace(/[\]]/g, "") with title.replace(/[\[\]]/g, "") (or escape: title.replace(/([\[\]])/g, "\\$1")).

@github-project-automation github-project-automation Bot moved this from Backlog to In progress in Apollon Development May 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In progress

Development

Successfully merging this pull request may close these issues.

2 participants